ping @JKingsnorth @eileenmcnaughton

Another simplest alternative to fix that regression would be to retain the $mode parameter of getRecipients fn. But that would be too manual approach in code pov as we always need to rely on an extra parameter to differentiate the mode rather than using mailing.sms_provider_id

If this PR is approved than I will create a user-doc PR to highlight the new changes in UI screen (that is make changes in https://github.com/civicrm/civicrm-user-guide/blob/master/docs/sms-text-messaging/everyday-tasks.md file)

The code looks logical on a quick look - I'm hoping @JKingsnorth can confirm since he is the one who has been working with you this

Jenkins, retest this please.

This seems like a reasonable approach. The reason I didn't do this before is because I think some extensions are tied in to this form process to provide custom functionality for SMS providers. I wanted to try to avoid making any significant changes to the process in order to fix the regression.

But it this is the preferred approach then OK [= I think @michaelmcandrew might have more experience with this area of the system than me, and I might not get a chance to look at this in detail today.

Also pinging @mattwire who's done work with SMS recently.

@monishdeb @JKingsnorth if there is doubt there is also the option of merging @JKingsnorth one in time for 4.7.31 & continuing on with this cleaner one to hit 4.7.32?

I haven't created (and can't see any extensions published in the extension directory - at least not ones with SMS in the title) that change that screen so I would say that it is fine.

Tangent: I don't really think the option should be there anyway - how many people use more than one SMS provider?

I guess it's there because it is 'possible' to add more than one provider. We can still default this to the main provider (as per my code in the other PR - but in the setDefaults instead).

Tangent: I don't really think the option should be there anyway - how many people use more than one SMS provider?

I agree with @michaelmcandrew During the fix I was wondering what could have been the requirement of having multiple SMS providers and that you can set them all by default.
Two alternatives:

1. Allow only one sms provider which on submission will be available as
   Civi::settings->get('sms_provider_id') and we no longer need to provide option in SMS composer screen.
2. Allow multiple but consider only one as default and that one should be set as the default in 'Select Recipients' screen

Jenkins test this please

2. is my preference, as per my previous comment.

I don't see the benefit of stripping out the functionality that allows for multiple SMS providers since it's already in place. One use case might be if you're using different SMS providers for different world regions.

Implementing #2 (or either #1) is not relevant to this issue, rather can be treated as an improvement. In my opinion, we need to fix it under a separate JIRA ticket. As this PR fixes the regression I think we need to get this PR merged ASAP after review as 4.7.31-rc branch is soon going to be cut.

What do you say guys?

Jenkins re test this please

Implementing #2 (or either #1) is not relevant to this issue

Yes - agree that it is tangential - didn't mean to sidetrack the issue. I don't see any reason to not go with this change.

There seems to be soft consensus on this & as it addresses a immediate regression I'm going to merge it to the rc. Please continue the discussion on & if need be agree further patches for the rc or master

Thanks @eileenmcnaughton